Skip to content

Conversation

@whqtker
Copy link
Member

@whqtker whqtker commented Jun 11, 2025

관련 이슈

작업 내용

  • SiteUser 엔티티의 nickname 컬럼에 unique 제약 조건을 추가하였고, 이에 따라 데이터베이스 컬럼 제약 조건을 수정하는 sql문을 작성하였습니다. 명시적으로 제약 조건 이름을 설정하였습니다. 또한 닉네임 중복 시의 예외를 추적할 수 있도록 하였습니다.
  • unique 제약 조건 위반으로 인해 발생할 수 있는 DataIntegrityViolationException 예외를 처리하는 로직을 try-catch 문으로 작성하였습니다. DB 레벨에서 발생할 수 있는 무결성 예외 처리를 CustomExceptionHandler 에 작성하였습니다.
  • 프로필 사진과 닉네임을 업데이트하는 로직을 수정하였습니다. 엔티티를 저장하는 것이 아닌 특정 컬럼의 값을 업데이트하는 쿼리를 작성하여 수정하였습니다. 준영속 엔티티에 save 메서드를 수행하여 N+1 문제가 발생하는 문제를 해결하였습니다.
  • 로직 변경에 따라 테스트 코드를 수정하였습니다.

특이 사항

취소선 다음 내용이 리뷰 피드백을 반영하여 새롭게 작업한 내용입니다!

리뷰 요구사항 (선택)

  • 코드 컨벤션을 잘 준수하고 있나요?
  • DataIntegrityViolationException 에서 구체적인 예외 내용을 얻기 위해 determineErrorCode 메서드를 사용하였습니다. 중복 코드가 될 거 같은데, 더 좋은 방법이 있을까요?

whqtker added 4 commits May 29, 2025 16:58
- SiteUser 엔티티의 nickname 컬럼에 unique 제약 조건 추가
- MyPageService에서 중복 닉네임 검증 로직 제거
- 컨트롤러에서 try-catch 문을 통해 예외 처리
- 변경된 컬럼만 업데이트하는 쿼리 사용
@whqtker whqtker self-assigned this Jun 11, 2025
@coderabbitai
Copy link

coderabbitai bot commented Jun 11, 2025

Walkthrough

  1. 새로운 에러 코드 및 예외 처리 추가

    • 데이터베이스 무결성 제약조건 위반을 나타내는 DATA_INTEGRITY_VIOLATION 에러 코드가 ErrorCode enum에 추가되었습니다.
    • CustomExceptionHandlerDataIntegrityViolationException을 처리하는 메서드가 새로 추가되어, 해당 예외 발생 시 적절한 에러 메시지와 상태 코드(409 Conflict)를 반환합니다.
  2. 닉네임 컬럼 고유 제약 조건 도입

    • SiteUser 엔티티에 닉네임의 유일성을 보장하는 고유 제약 조건이 추가되었습니다.
    • 이에 따라 데이터베이스 마이그레이션 스크립트(V14__set_unique_constraint_to_nickname.sql)도 추가되어, 실제 DB에도 제약 조건이 반영됩니다.
  3. 마이페이지 서비스 내부 로직 개선

    • MyPageServiceupdateMyPageInfo 메서드에서 사용자 엔티티를 직접 받아 사용하던 방식에서, ID로 엔티티를 조회하여 사용하는 방식으로 변경되었습니다.
    • 닉네임 및 프로필 이미지 변경 시에도 항상 최신 엔티티를 사용하도록 수정되었습니다.
    • 닉네임 중복 검사 메서드의 위치가 클래스 내에서 이동되었습니다.
  4. 테스트 코드의 닉네임 고유성 반영 및 리팩토링

    • 테스트용 사용자 생성 메서드가 닉네임을 파라미터로 받도록 변경되었습니다.
    • 닉네임 중복 저장 및 변경 시 예외가 발생하는지 검증하는 테스트가 추가되었습니다.
    • 마이페이지 서비스 테스트에서는 닉네임 중복 변경 시 예외 테스트가 제거되고, 프로필 이미지 변경 검증 로직이 개선되었습니다.
  5. 기타 코드 정리

    • 일부 파일(MyPageController.java, SiteUserRepository.java)에서 패키지 선언 후 공백 줄이 추가되어 코드 스타일이 정돈되었습니다.

Suggested reviewers

  • nayonsoso
  • wibaek

📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 16e55f7 and 6265684.

📒 Files selected for processing (1)
  • src/main/resources/db/migration/V14__set_unique_constraint_to_nickname.sql (1 hunks)
✅ Files skipped from review due to trivial changes (1)
  • src/main/resources/db/migration/V14__set_unique_constraint_to_nickname.sql
⏰ Context from checks skipped due to timeout of 90000ms (1)
  • GitHub Check: build

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@whqtker whqtker changed the title refactor/Siteuser 도메인 리팩터링 refactor: Siteuser 도메인 리팩터링 Jun 11, 2025
- nickname 제약 조건의 이름 명시적으로 지정
@whqtker whqtker marked this pull request as ready for review June 12, 2025 06:20
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 6

🧹 Nitpick comments (2)
src/main/java/com/example/solidconnection/auth/service/SignUpService.java (1)

70-74: 3) 더 이상 호출되지 않는 메서드 정리 필요

validateNicknameDuplicated 는 현재 서비스 로직에서 호출되지 않습니다.
Dead code 는 유지보수를 방해하므로 삭제를 권장합니다.

src/main/java/com/example/solidconnection/siteuser/service/MyPageService.java (1)

51-61: 2) JPQL bulk update 후 영속성 컨텍스트 불일치

updateNickname/updateProfileImage 는 JPQL UPDATE 이므로 1차 캐시에 남아있는 siteUser 엔티티의 필드가 갱신되지 않습니다.
메서드 종료 이후 동일 세션에서 엔티티를 다시 사용할 경우 데이터가 오래된 상태로 남아있어 의도치 않은 버그가 발생할 수 있습니다.

  1. EntityManager.clear() 혹은 refresh(siteUser) 호출
  2. or 메서드 매개변수로 엔티티 대신 ID 만 받도록 변경

중 하나로 일관성을 확보해 주세요.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ac0f60e and 6f7cf1f.

📒 Files selected for processing (7)
  • src/main/java/com/example/solidconnection/auth/service/SignUpService.java (4 hunks)
  • src/main/java/com/example/solidconnection/common/exception/ErrorCode.java (1 hunks)
  • src/main/java/com/example/solidconnection/siteuser/controller/MyPageController.java (3 hunks)
  • src/main/java/com/example/solidconnection/siteuser/domain/SiteUser.java (1 hunks)
  • src/main/java/com/example/solidconnection/siteuser/repository/SiteUserRepository.java (2 hunks)
  • src/main/java/com/example/solidconnection/siteuser/service/MyPageService.java (2 hunks)
  • src/main/resources/db/migration/V13__set_unique_constraint_to_nickname.sql (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
  • GitHub Check: build
🔇 Additional comments (3)
src/main/java/com/example/solidconnection/common/exception/ErrorCode.java (1)

99-101: 🎉 신규 에러코드 추가, 문제 없습니다

DATA_INTEGRITY_VIOLATION 상수의 코드·메시지·HTTP 상태값이 기존 규칙과 일관성을 유지합니다. 다른 계층에서 재활용하기에 충분히 명확해 보여요. 수고하셨습니다!

src/main/resources/db/migration/V13__set_unique_constraint_to_nickname.sql (1)

1-3: 1️⃣ 운영 DB에 적용 전, 중복 데이터 존재 여부 꼭 확인하세요

이미 중복 닉네임이 저장돼 있다면 마이그레이션이 실패해 배포가 중단됩니다. 배포 직전에 다음 쿼리를 한 번 실행해 안전을 확보해 주세요.

SELECT nickname, COUNT(*)
FROM site_user
GROUP BY nickname
HAVING COUNT(*) > 1;

중복 레코드가 있다면 미리 정리하거나 임시 처리 정책을 마련하는 것을 권장드립니다.

src/main/java/com/example/solidconnection/siteuser/domain/SiteUser.java (1)

38-42: 2️⃣ 닉네임 유니크 제약 추가 확인

엔티티에 제약 명시까지 반영된 점 아주 좋습니다. DB-스키마와 JPA 설정이 싱크를 맞추도록 명시적 이름을 사용한 것도 👍🏻 입니다.

@whqtker whqtker marked this pull request as draft June 12, 2025 06:45
- 게시글_좋아요_동시성_문제를_해결한다: 동일한 닉네임을 사용하지 않도록 변경
- 이메일이_같더라도_인증_유형이_다른_사용자는_정상_저장한다: 동일한 닉네임을 사용하지 않도록 변경
- 닉네임 무결성 테스트는 repository test로 분리
- 새로운_이미지로_성공적으로_업데이트한다: DB에서 사용자 조회 후 URL 비교
@whqtker whqtker marked this pull request as ready for review June 12, 2025 08:32
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
src/test/java/com/example/solidconnection/concurrency/PostLikeCountConcurrencyTest.java (1)

24-24: 대량 사용자 생성 루프의 트랜잭션 비용 검토 제안

  1. 현재 save 호출을 1,000 번 반복하면서 각 건마다 즉시 flush 하지 않더라도 커넥션 풀 소진·성능 저하 가능성이 있습니다.
  2. saveAll + 배치 flush 또는 EntityManager level batch size 설정으로 테스트 시간을 줄일 수 있습니다.
  3. 테스트 목적에 영향 없으므로 성능을 조금 아껴보시는 건 어떨까요?

Also applies to: 95-97

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6f7cf1f and c8115f3.

📒 Files selected for processing (4)
  • src/test/java/com/example/solidconnection/concurrency/PostLikeCountConcurrencyTest.java (2 hunks)
  • src/test/java/com/example/solidconnection/e2e/DynamicFixture.java (1 hunks)
  • src/test/java/com/example/solidconnection/siteuser/repository/SiteUserRepositoryTest.java (3 hunks)
  • src/test/java/com/example/solidconnection/siteuser/service/MyPageServiceTest.java (1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/test/java/com/example/solidconnection/concurrency/PostLikeCountConcurrencyTest.java (1)
src/test/java/com/example/solidconnection/e2e/DynamicFixture.java (1)
  • DynamicFixture (7-18)
🔇 Additional comments (3)
src/test/java/com/example/solidconnection/siteuser/service/MyPageServiceTest.java (1)

121-122: 영속 상태로 재확인하도록 수정된 검증 👍
저장소에서 다시 조회해 비교하도록 바꾼 덕분에 실제 DB 반영 여부까지 확인할 수 있습니다. 테스트 신뢰도가 한 단계 올라갔네요.

src/test/java/com/example/solidconnection/e2e/DynamicFixture.java (1)

9-16: 닉네임 파라미터 추가로 유틸 완성도 향상 🎉
메서드 명세와 내부 구현이 일관성을 되찾았습니다. 고정 문자열 대신 전달받은 nickname을 써서 중복 제약 테스트에 잘 맞춰졌네요.

src/test/java/com/example/solidconnection/siteuser/repository/SiteUserRepositoryTest.java (1)

73-78: 중복 닉네임 변경 검증 👍
엔티티 업데이트 후 saveAndFlush 로 즉시 검증한 부분이 모범적입니다. 제약 위반을 확실히 끌어내는 좋은 예시네요.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (5)
src/test/java/com/example/solidconnection/siteuser/service/MyPageServiceTest.java (1)

121-121: Optional: get() 대신 orElseThrow() 사용 제안

findById(...).get()은 값이 없을 때 NoSuchElementException을 던지지만, 메시지가 불명확합니다. orElseThrow()를 쓰면 예외 유형과 메시지를 선택적으로 제어할 수 있어 테스트 의도를 더 분명히 드러낼 수 있습니다.

- SiteUser updatedUser = siteUserRepository.findById(user.getId()).get();
+ SiteUser updatedUser = siteUserRepository.findById(user.getId()).orElseThrow();
src/test/java/com/example/solidconnection/e2e/DynamicFixture.java (1)

9-13: Fixture 시그니처 확장 👍, 하지만 AuthType 주입 고려

  1. 닉네임 인자를 받아 유니크 제약을 쉽게 우회할 수 있게 됐습니다.
  2. 다만 실제 도메인에서 AuthType이 필수라면, 동일한 메서드 오버로드로 AuthType까지 받도록 확장하면 테스트 유연성이 더 좋아집니다.
src/test/java/com/example/solidconnection/siteuser/repository/SiteUserRepositoryTest.java (3)

27-33: saveAndFlush로 즉시 예외 발생 보장

save()만 호출하면 플러시 시점이 트랜잭션 종료까지 지연될 수 있어, 예상한 DataIntegrityViolationException이 바로 발생하지 않을 위험이 있습니다. 테스트 안정성을 위해 아래와 같이 변경을 권장합니다.

- siteUserRepository.save(user1);
+ siteUserRepository.saveAndFlush(user1);
...
- assertThatCode(() -> siteUserRepository.save(user2))
+ assertThatCode(() -> siteUserRepository.saveAndFlush(user2))

39-45: 중복 이메일-다른 AuthType 검증도 동일 패턴으로 통일

위와 같은 이유로 이곳도 saveAndFlush를 사용하면 일관성과 즉시성 두 마리 토끼를 잡을 수 있습니다.


50-79: 닉네임 중복 테스트: 단계별 flush 권장

  1. user1 저장 후 곧바로 flush → 제약이 실제 DB에 반영
  2. user2 저장 시 중복 여부가 즉각 확인
- siteUserRepository.save(user1);
+ siteUserRepository.saveAndFlush(user1);

같은 이유로 저장-후-변경 케이스도 첫 저장 단계에서 flush 해두면 테스트 의도가 더 명확해집니다.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6f7cf1f and c8115f3.

📒 Files selected for processing (4)
  • src/test/java/com/example/solidconnection/concurrency/PostLikeCountConcurrencyTest.java (2 hunks)
  • src/test/java/com/example/solidconnection/e2e/DynamicFixture.java (1 hunks)
  • src/test/java/com/example/solidconnection/siteuser/repository/SiteUserRepositoryTest.java (3 hunks)
  • src/test/java/com/example/solidconnection/siteuser/service/MyPageServiceTest.java (1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/test/java/com/example/solidconnection/concurrency/PostLikeCountConcurrencyTest.java (1)
src/test/java/com/example/solidconnection/e2e/DynamicFixture.java (1)
  • DynamicFixture (7-18)
🔇 Additional comments (2)
src/test/java/com/example/solidconnection/siteuser/service/MyPageServiceTest.java (1)

121-122: 엔티티 재조회로 검증 정확도 향상

  1. 서비스 호출 후 영속성 컨텍스트에 남아 있는 객체 대신 DB에서 새로 조회하여 상태 동기화 문제를 제거했습니다.
  2. 변경이 의도대로 저장됐는지 확실히 검증하므로 테스트 신뢰도가 상승했습니다.
src/test/java/com/example/solidconnection/concurrency/PostLikeCountConcurrencyTest.java (1)

95-97: 닉네임 파라미터 추가로 유니크 제약 대응 완료

이제 이메일과 닉네임을 모두 고유하게 생성하므로 DB 제약 위반 가능성이 제거되었습니다. 테스트가 안정적으로 수행됩니다.

Copy link
Contributor

@Gyuhyeok99 Gyuhyeok99 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

고생하셨습니다! 커밋 따라서 잘 이해되게 작성해주셔서 금방 읽었습니다!
궁금한 점은 리뷰로 남겨놓았습니다 😊

whqtker added 2 commits June 15, 2025 10:20
- 중복 닉네임 검증보다 닉네임 수정 시간 검증이 선행되도록 변경
- 준영속 엔티티를 save()하는 과정에서 발생하는 N+1 문제 해결
- DB 레벨, 애플리케이션 레벨 검증 모두 사용하도록
- 위 변경사항에 따른 테스트 코드 수정
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between afdf86f and 074ca56.

📒 Files selected for processing (3)
  • src/main/java/com/example/solidconnection/common/exception/CustomExceptionHandler.java (3 hunks)
  • src/main/java/com/example/solidconnection/siteuser/service/MyPageService.java (3 hunks)
  • src/test/java/com/example/solidconnection/siteuser/service/MyPageServiceTest.java (4 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • src/test/java/com/example/solidconnection/siteuser/service/MyPageServiceTest.java
  • src/main/java/com/example/solidconnection/siteuser/service/MyPageService.java
⏰ Context from checks skipped due to timeout of 90000ms (1)
  • GitHub Check: build
🔇 Additional comments (2)
src/main/java/com/example/solidconnection/common/exception/CustomExceptionHandler.java (2)

7-7: 👍 신규 예외 타입 추가 확인 완료

DataIntegrityViolationException 임포트가 잘 반영되어 있습니다. 다른 클래스에서 동일 예외를 직접 핸들링하지 않고 전역 핸들러로 일원화된 점도 좋습니다.


17-17: 코드 일관성 유지에 도움되는 static import

DATA_INTEGRITY_VIOLATION 상수에 대한 static import는 가독성을 높여줍니다. 별다른 이슈 없습니다.

@whqtker
Copy link
Member Author

whqtker commented Jun 15, 2025

074ca56 에 대한 추가 설명입니다.

  • 기존 N+1 문제가 준영속 엔티티를 merge() 하는 과정에서 발생한 것이므로, 영속 상태 엔티티를 새로 조회하고 수정하도록 구현하여 dirty checking이 효율적으로 동작하도록 하였습니다.
  • 예외 처리 로직을 컨트롤러 계층이 아닌 서비스 계층에서 처리하도록 변경하려고 했으나, DB 레벨 검증, 애플리케이션 검증 모두 서비스 계층에서 처리하게 되면 코드 복잡성이 증가하였습니다. 따라서 서비스 계층에 애플리케이션 예외 처리를 다시 추가하고, CustomExceptionHandler 에 무결성 관련 예외 처리를 추가하였습니다.
    • 이전 변경사항에서 DB 레벨 검증'만' 진행하는 걸로 생각해서 DataIntegrityViolationException 에서 문자열을 파싱하여 닉네임 중복 예외 처리하도록 구현했습니다. 그러나 기존에 존재했던 애플리케이션 레벨 검증(validateNicknameDuplicated)을 살리는 방향으로 결정되어서 DB 레벨 검증은 DataIntegrityViolationException 예외 자체만 다루는 것으로 작성하였습니다!

기존 코드와의 변경 사항을 정리하면 다음과 같습니다.

  1. MyPageServiceupdateMyPageInfo: save() 제거, 영속 상태 엔티티 조회 로직 추가 -> dirty checking 을 사용하면서 N+1 문제도 방지.
  2. DB 레벨 예외 처리를 CustomExceptionHandler 에 추가. -> 결과적으로 애플리케이션, DB 레벨 검증 모두 사용.

updateMyPageInfo 메서드에서 엔티티를 새로 조회하는 로직이 추가됨에 따라 테스트 코드를 일부 변경하였습니다. 이에 대한 추가 설명입니다.

  • 최소_대기기간이_지나지_않은_상태에서_변경하면_예외_응답을_반환한다: setter로 변경한 값이 DB에 반영되지 않았고, 따라서 변경한 값을 명시적으로 DB에 반영되도록 수정하였습니다.
  • 프로필을_처음_수정하는_것이_아니라면_이전_이미지를_삭제한다: 엔티티를 새로 조회하므로 객체 동일성으로 검증할 수 없습니다.

Copy link
Collaborator

@nayonsoso nayonsoso left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

어떤 부분이 변경되었는지를 잘 나타내주셔서 따라가며 읽기 편했습니다😁😁😁 감사합니다.

P.S 작업 내용에 대한 것은 아니지만 커밋에 대해 피드백이 있습니다.
개인적으로는 커밋도 팀의 자산이라 생각해서 말씀드려봅니다.!

  • 커밋의 제목은 '작업 대상'이 아니라 '작업 내용'이 담겨있는게 좋겠습니다.
    • 389ce9b 같은 경우는 커밋 제목이 "refactor: nickname unique 제약 추가" 같은게 좋겠습니다.
  • 0699152 커밋의 커밋 프리픽스는 fix가 아니라 chore이 적당해보입니다. fix는 버그를 수정했을 때만 쓴다고 하더라고요.

Comment on lines +60 to +61
assertThatCode(() -> siteUserRepository.saveAndFlush(user2))
.isInstanceOf(DataIntegrityViolationException.class);
Copy link
Collaborator

@nayonsoso nayonsoso Jun 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

save에서 saveAnsFlush()로 바꾸신 이유가 궁금해요!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DB 레벨 테스트는 save 대신 saveAndFlush를 사용하였습니다. 아무래도 DB에 반영시킨 후의 결과를 봐야 하니까요. 물론 중복된_닉네임으로_사용자를_저장하면_예외_응답을_반환한다 테스트는 save 를 사용해도 통과가 되지만, DB 레벨 테스트라 saveAndFlush를 사용하였습니다. 아무래도 성능 측면에서는 save가 더 좋긴 할텐데, 어떤 기준으로 save, saveAndFlush를 선택해야 할지 고민이 되네요 ...

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

아무래도 DB에 반영시킨 후의 결과를 봐야 하니까요.

아무래도 성능 측면에서는 save가 더 좋긴 할텐데,

이 부분에 대해서 제가 아는 내용으로는, (아니라면 지적해주세요!)
우리 테스트 코드에서는 Transactional 어노테이션을 사용하지 않고 있기 때문에,
assert문 내부에 save와 saveAndFlush 의 동작은 실질적으로 차이가 없다고 알고 있습니다!

물론 Transactional 어노테이션이 있는 테스트 코드라면 반드시 saveAndFlush를 써야 assert문에 걸리겠지만요 ㅎㅎ


어떤 기준으로 save, saveAndFlush를 선택해야 할지 고민이 되네요 ...

저도 고민이 되네요.. 🤔 우리 컨벤션대로라면 save를 써도 될텐데,.
미래에 Transactional 어노테이션을 쓸 상황을 대비해서 saveAndFlush로 방어적인 코드를 작성해야할지...?
개인적으로 saveAndFlush를 쓰는게 더 좋겠다는 생각은 듭니다!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

그렇군요 .. @Transaction 이 붙은 테스트 코드 작성 시에는 주의해야겠네요.

저는 방어적으로 작성한다고 손해보지는 않는다고 생각합니다 ㅎㅎ

@whqtker
Copy link
Member Author

whqtker commented Jun 18, 2025

어떤 부분이 변경되었는지를 잘 나타내주셔서 따라가며 읽기 편했습니다😁😁😁 감사합니다.

P.S 작업 내용에 대한 것은 아니지만 커밋에 대해 피드백이 있습니다. 개인적으로는 커밋도 팀의 자산이라 생각해서 말씀드려봅니다.!

  • 커밋의 제목은 '작업 대상'이 아니라 '작업 내용'이 담겨있는게 좋겠습니다.

    • 389ce9b 같은 경우는 커밋 제목이 "refactor: nickname unique 제약 추가" 같은게 좋겠습니다.
  • 0699152 커밋의 커밋 프리픽스는 fix가 아니라 chore이 적당해보입니다. fix는 버그를 수정했을 때만 쓴다고 하더라고요.

좋은 리뷰 감사합니다~ 피드백 주신 내용들 꼼꼼히 읽고 답변드리겠습니다!
커밋 컨벤션 관련 피드백도 정말 감사합니다! 습관적으로 작성해서 잘 인지하지 못했습니다 ㅠㅠ

Copy link
Collaborator

@nayonsoso nayonsoso left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

코멘트 달아주신 부분 다 확인했습니다 😄
저는 이번에 resolve 하지 않은 부분에 추가 코멘트만 달았습니다~
수고하셨습니다!!

@whqtker whqtker merged commit af2ba81 into solid-connection:develop Jun 24, 2025
2 checks passed
* */
@Transactional
public void updateMyPageInfo(SiteUser siteUser, MultipartFile imageFile, String nickname) {
SiteUser user = siteUserRepository.findById(siteUser.getId())
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

메서드 파라미터로 SiteUser 객체 전달 관련 이슈: #299

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

refactor: SiteUser 도메인 리팩터링

3 participants